-
Notifications
You must be signed in to change notification settings - Fork 238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implementing soft delete for storage bucket #2051
Implementing soft delete for storage bucket #2051
Conversation
/lgtm PR looks good. @600lyy could you fix the presubmit check? |
permanently deleted. If it is not provided, by default Google Cloud | ||
Storage sets this to default soft delete policy. | ||
properties: | ||
effectiveTime: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yuwenma this field is an output-only field that gets inserted under spec
. According to our previous discussion, we don't want to add any output-only spec fields any more - is my understanding correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a workaround to hide output-only spec field but still support it under observed state: #2075 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @maqiuyujoyce for the quick workaround!
@@ -23,3 +23,5 @@ spec: | |||
versioning: | |||
enabled: true | |||
lifecycleRule: [] | |||
softDeletePolicy: | |||
retentionDurationSeconds: 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran the test locally and found that 0
was not used in the update, and the value kept to be 604800
. Could you verify that this is a valid update; or whether setting the value to 0
is actually effective? AFAIK, we don't support setting a primitive-typed field to its empty value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked again and actually it worked.
Add support for bucket's soft delete policy which defines the period of time that soft-deleted objects will be retained
Looks like I can do the rebase / cleanup; just pushed to the PR branch 🤞 |
Looks like a (simple) rebase was all that was needed - thank you @600lyy /approve @maqiuyujoyce I think we should merge this before #2075 |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ddd7cd3
into
GoogleCloudPlatform:master
Change description
Add support for soft delete policy for Google storage bucket.
The bucket's soft delete policy, which defines the period of time that soft-deleted objects will be retained, and cannot be permanently deleted. If it is not provided, by default Google Cloud Storage sets this to default soft delete policy`
This is the implementation for #2046
make ready-pr
to ensure this PR is ready for review.